Skip to content

repeat_row: Add comments around snapshot_latest, documenting the set invariant#36191

Open
ggevay wants to merge 1 commit intoMaterializeInc:mainfrom
ggevay:repeat_row-6-snapshot_latest
Open

repeat_row: Add comments around snapshot_latest, documenting the set invariant#36191
ggevay wants to merge 1 commit intoMaterializeInc:mainfrom
ggevay:repeat_row-6-snapshot_latest

Conversation

@ggevay
Copy link
Copy Markdown
Contributor

@ggevay ggevay commented Apr 21, 2026

This is another PR in the repeat_row workstream. I just wanted to make sure that repeat_row with a negative count can't possibly make the assert_eq!(diff, 1, "snapshot doesn't accumulate to set"); in snapshot_latest fire. This seems to be the case, since this is reading a collection that is not a user-collection, so data from repeat_row can't reach it. (In fact, the set invariant is a much stronger invariant.)

This PR just adds code comments documenting the set invariant, and why it holds at the only caller of snapshot_latest.

Resolves SQL-171.

@ggevay ggevay requested review from a team as code owners April 21, 2026 18:15
@ggevay ggevay added the A-ADAPTER Topics related to the ADAPTER layer label Apr 21, 2026
@ggevay ggevay requested a review from ohbadiah April 21, 2026 18:15
@ggevay ggevay force-pushed the repeat_row-6-snapshot_latest branch from d72bac7 to b527c34 Compare April 21, 2026 18:24
let as_of = f.step_back().unwrap();

let snapshot = self.snapshot(id, as_of, &self.txns_read).await.unwrap();
let snapshot = self.snapshot(id, as_of, &self.txns_read).await?;
Copy link
Copy Markdown
Contributor Author

@ggevay ggevay Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so important for the main point of the PR, but I did it because why unwrap when you can just ?-propagate the same error?

@ggevay ggevay force-pushed the repeat_row-6-snapshot_latest branch from b527c34 to 5af8d24 Compare April 21, 2026 18:28
@ggevay ggevay force-pushed the repeat_row-6-snapshot_latest branch from 5af8d24 to 80a81d7 Compare April 21, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ADAPTER Topics related to the ADAPTER layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant